Compiler warning: use of `mktemp` is dangerous. Fixes #956 by pmqs · Pull Request #957 · zlib-ng/minizip-ng

pmqs · 2026-02-04T14:17:17Z

Silence the compiler warning below by using mkdtemp instead of mktemp

57%] Linking C executable minigzip
/usr/bin/ld: libminizip.a(mz_os_posix.c.o): in function `mz_os_get_temp_path':
/home/runner/work/minizip-ng/minizip-ng/mz_os_posix.c:374:(.text+0x1023): warning: the use of `mktemp' is dangerous, better use `mkstemp' or `mkdtemp'
[ 57%] Built target minigzip_cli
[ 60%] Building C object CMakeFiles/minizip_cli.dir/minizip.c.o
[ 62%] Linking C executable minizip
/usr/bin/ld: libminizip.a(mz_os_posix.c.o): in function `mz_os_get_temp_path':
/home/runner/work/minizip-ng/minizip-ng/mz_os_posix.c:374:(.text+0x1023): warning: the use of `mktemp' is dangerous, better use `mkstemp' or `mkdtemp'

nmoinvaz · 2026-02-04T15:13:35Z

mz_os_posix.c

        return MZ_INTERNAL_ERROR;

+    /* Create a filename inside the temporary directory */
+    result = snprintf(path, max_path, "%s/f", temp_path);


The filename is f?

The filename is f?

Yes.

tbeu · 2026-02-06T18:54:13Z

mz_os_posix.c

+    /* Check for no environment variable set at all */
+    if (!tmp_dir)
+        return MZ_INTERNAL_ERROR;
+


Suggested change

/* Check for no environment variable set at all */

if (!tmp_dir)

return MZ_INTERNAL_ERROR;

Is this reachable actually?

Good catch. I missed the last if statement in the block of code before this. I'll update it

if (!tmp_dir) tmp_dir = "/tmp";

nmoinvaz · 2026-02-06T23:26:25Z

Maybe I am a dumb-dumb, but why is random dir with a fixed name more secure than a random filename with a fixed dir?

pmqs · 2026-02-07T09:01:40Z

Maybe I am a dumb-dumb, but why is random dir with a fixed name more secure than a random filename with a fixed dir?

I believe that mktemp is deprecated because it doesn't create a the temporary file in the filesystem, so there is a potential race condition where a bad actor guesses the filename before you get a change to use it.

mkdtemp atomically creates the directory, so no race condition.

Coeur · 2026-02-09T06:27:06Z

BUGS

  Never use mktemp().  Some implementations follow 4.3BSD and
  replace XXXXXX by the current process ID and a single letter, so
  that at most 26 different names can be returned.  Since on the one
  hand the names are easy to guess, and on the other hand there is a
  race between testing whether the name exists and opening the file,
  every use of mktemp() is a security risk.  The race is avoided by
  [mkstemp(3)](https://man7.org/linux/man-pages/man3/mkstemp.3.html) and [mkdtemp(3)](https://man7.org/linux/man-pages/man3/mkdtemp.3.html).

Source: https://man7.org/linux/man-pages/man3/mktemp.3.html

nmoinvaz · 2026-02-09T06:40:03Z

@Coeur thanks that makes sense.

I suggest instead of f for filename we use snprintf with just the unix timestamp.

…zlib-ng#957

pmqs · 2026-02-09T10:02:24Z

@Coeur thanks that makes sense.

I suggest instead of f for filename we use snprintf with just the unix timestamp.

Done

nmoinvaz · 2026-02-09T19:24:32Z

mz_os_posix.c

 int32_t mz_os_get_temp_path(char *path, int32_t max_path, const char *prefix) {
    const char *tmp_dir = NULL;
    int32_t result = 0;
+    char temp_path[max_path];


Sorry for this, but I think this is a newer C standard thing right? Maybe just best to do calloc().

C99 brought stack-allocated variable-length arrays, see https://en.wikipedia.org/wiki/Variable-length_array#C.

Yes, better stick with heap-allocated array here.

Ha! I knew that zlib-ng policed this one in cmake and assumed minizip-ng did as well.

I've made the change, but can we get cmake to police a minimum version?

nmoinvaz · 2026-02-09T23:06:06Z

mz_os_posix.c


 int32_t mz_os_get_temp_path(char *path, int32_t max_path, const char *prefix) {
    const char *tmp_dir = NULL;
+    char *temp_path = (char *)calloc(max_path, sizeof(char));


char *temp_path = NULL;

nmoinvaz · 2026-02-09T23:07:28Z

mz_os_posix.c


-    /* Build template path for mktemp: <tmp_dir>/<prefix>XXXXXX */
-    result = snprintf(path, max_path, "%s/%sXXXXXX", tmp_dir, prefix ? prefix : "");
+    /* Build template path for mkdtemp: <tmp_dir>/<prefix>XXXXXX */


temp_path = (char *)calloc(max_path, sizeof(char));

Don't forget to do free(temp_path) also for every failure path after.

temp_path = (char *)calloc(max_path, sizeof(char));

Don't forget to do free(temp_path) also for every failure path after.

Wow! I run with address sanitizer enabled all the time to catch things like this.

Hmm, just looking at the compiler commandline I'm not seeing the expected options.

Anyway, I've added the free

This is still not fixed, there are return paths where it would leak memory.

…lib-ng#957

Compiler warning: use of mktemp is dangerous. Fixes zlib-ng#956

b2bd5f6

nmoinvaz reviewed Feb 4, 2026

View reviewed changes

tbeu reviewed Feb 6, 2026

View reviewed changes

Remove redundant test for no temp_dir

6dd9270

fix: update temporary file naming to include timestamp for uniqueness. …

d887b15

…zlib-ng#957

nmoinvaz reviewed Feb 9, 2026

View reviewed changes

Change a C99 dynamic stack allocation into a calloc'ed heap allocation

5547931

nmoinvaz requested changes Feb 9, 2026

View reviewed changes

pmqs added 3 commits February 10, 2026 10:40

fix: free allocated memory for temporary path in mz_os_get_temp_path z…

743b41e

…lib-ng#957

clang-format: remove extra space in free call in mz_os_get_temp_path z…

674c22d

…lib-ng#957

add more frees

47ef7fb

	/* Check for no environment variable set at all */
	if (!tmp_dir)
	return MZ_INTERNAL_ERROR;

Uh oh!

Conversation

pmqs commented Feb 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nmoinvaz commented Feb 6, 2026

Uh oh!

pmqs commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Coeur commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nmoinvaz commented Feb 9, 2026

Uh oh!

pmqs commented Feb 9, 2026

Uh oh!

nmoinvaz Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pmqs commented Feb 7, 2026 •

edited

Loading

Coeur commented Feb 9, 2026 •

edited

Loading

nmoinvaz Feb 9, 2026 •

edited

Loading